Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate readandwrite and add docs for Pipe #24718

Merged
merged 2 commits into from
Jan 17, 2018
Merged

Deprecate readandwrite and add docs for Pipe #24718

merged 2 commits into from
Jan 17, 2018

Conversation

Keno
Copy link
Member

@Keno Keno commented Nov 23, 2017

Came up here: https://discourse.julialang.org/t/document-process-interaction-with-stdin-stout-stderr/7251/3.

Also document what Pipe() does, though note that this API may have to change due to #24717.

@ararslan ararslan added deprecation This change introduces or involves a deprecation docs This change adds or pertains to documentation io Involving the I/O subsystem: libuv, read, write, etc. labels Nov 23, 2017
mbaz
mbaz reviewed Nov 23, 2017
View reviewed changes
base/stream.jl Outdated
```
julia> err = Pipe()

# After this `p` will be initialized and you may read `foo`'s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is p here?

base/stream.jl Outdated

# After this `p` will be initialized and you may read `foo`'s
# stderr from the `err` pipe.
julia> spawn(pipeline(pipeline(`foo`, stderr=err), `cat`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command is missing a closing )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think we could also include an example with this syntax:

spawn(cat, stdin, stdout, stderr)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 4-argument spawn() appears to be undocumented though.

@mbaz mbaz mentioned this pull request Nov 27, 2017
Open
4 tasks
@JeffBezanson
Copy link
Member

Bump. Needs rebasing and doc string editing, and then should probably be merged for 1.0.

@JeffBezanson JeffBezanson added this to the 1.0 milestone Jan 15, 2018
Keno added 2 commits January 16, 2018 14:50
@Keno @JeffBezanson
Deprecate readandwrite
7577ec2 
There was a remaining todo in the code to deprecate this function.
Do so.
@Keno @JeffBezanson
Add a docstring showing supported usage of Pipe()
1755a3c 
It was brought up on discourse that this function doesn't have any docs.
@JeffBezanson JeffBezanson merged commit cb23538 into master Jan 17, 2018
@JeffBezanson JeffBezanson deleted the kf/deprw branch January 17, 2018 03:49
@stevengj
Copy link
Member

@Keno, p = open(cmd, "r+") still doesn't seem to be mentioned in the documention; am I missing something?

Also, the apparent need to close(p.in) before reading (if the program is still waiting for input) is a bit fiddly (see also this discourse thread). Not sure if there is a better way.

In fact, the docstring for open(cmd) still uses the deprecated open(command, stdio=devnull; write, read) API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbaz mbaz mbaz left review comments

@mortenpi mortenpi mortenpi left review comments

Assignees
No one assigned
Labels
deprecation This change introduces or involves a deprecation docs This change adds or pertains to documentation io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Milestone
0.7
Development

Successfully merging this pull request may close these issues.
6 participants
@Keno @JeffBezanson @stevengj @mortenpi @mbaz @ararslan